Skip to content

fix: type merged fields as child-wins, matching the runtime - #54

Merged
btravers merged 1 commit into
mainfrom
followup/merged-fields
Aug 9, 2026
Merged

fix: type merged fields as child-wins, matching the runtime#54
btravers merged 1 commit into
mainfrom
followup/merged-fields

Conversation

@btravers

@btravers btravers commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Retires the last recorded type lie in extend: merged fields were typed S & S2 while the runtime is { ...parent.fields, ...nextFields } — child wins per key. A variant redeclaring an inherited field under a different brand read as both brands while holding one. The identical lie was fixed for the computed map in the previous release (MergedComputed); this is its sibling, built to the same playbook.

The change

export type MergedFields<S extends Fields, S2 extends Fields> = Omit<S, keyof S2> & S2;

All five S & S2 positions in AbstractEntity["extend"] convert. The return type and the two InputOf positions carried the real lie — a rule's or derivation's d typed a redeclared field as both brands. The two keyof bounds were provably equivalent key sets (measured, including the symbol-key case), changed for single-spelling consistency; they are the safe revert if const-inference ever misbehaves.

The old "left as S & S2, known, why" comment is retired — its question now has a measured answer.

The emit playbook, followed first pass

The MergedComputed episode taught that the inline form emits a dangling type parameter into consumers' declarations under TS 5.9.3 (TS2304). MergedFields is a named export from the start, registered on all seven surfaces: the alias, index.ts's emit-nameability list, the *Src + namespace member, emit-guards.ts (with Record<never, never> as the argument shape that broke last time), typedoc.json, and types.md's four sub-surfaces. Review confirmed all seven and searched for an eighth — there is none.

Measured cost: the emitter writes the alias by reference; the consumer's index.d.ts grew 10,061 → 10,145 bytes — +42 per variant. The deferral reason ("TS7056 budget on every entity") is answered, not assumed: no TS7056, no TS2304, no TS4023 on either compiler.

What the honest surfaces are — same story as computed

A variant's instance type still shows the intersection for a redeclared key, because BehaviourOf<This> carries the root's instance unmapped (mapping it breaks abstract members, TS2425 — pinned). Entity.Output, toJSON() and output.shape are the honest surfaces, exactly as documented for redefined computed keys. declaration.md's caveat is generalised to redeclared keys in either map, and the option table gains a fields row ("merged per key"), kept clearly distinct from the concatenating lists.

Guards, all proven discriminating

  • Type: Entity.Output<typeof Variant>[key] is the variant's brand alone. The @ts-expect-error is proven load-bearing the honest way: restoring S & S2 yields exactly one diagnostic — TS2578 on the directive — and the comments say precisely that (an intersection assigns to each constituent, so the failure surfaces as the directive going unused, not as the positive line breaking). A pre-existing comment on the computed twin made the stronger, false claim; fixed here too.
  • Runtime: root Code5 (length(5)) vs child Digits (/^\d+$/) discriminates all three merge semantics — "abcde" rules out parent-wins, "42" rules out an intersect merge, "12345" proves the key is not dropped. Hand-verified in review.

Breaking

minor. No entity declaration that compiled stops compiling. Code consuming a redeclared key through the old intersection type may now fail to compile instead of passing silently — which is the honest type doing its job. The changeset says so, split exactly that way. Also folds in: CLAUDE.md's declaration-emit-names count corrected to nine (it understated by two even before this branch).

Test plan

  • 172 package tests (one new), test:types with every directive used, full six-step gate including the four-step consumer pass on 7.0.2 and 5.9.3 — green, uncached
  • Review measured the keyof-equivalence and brand-assignability claims independently rather than trusting the report

Known follow-up, deliberately not here: no direct pin on a redeclared field that is also immutable/generated on the root.

🤖 Generated with Claude Code

`extend` merges fields with `{ ...parent.fields, ...nextFields }`, so a
variant redeclaring an inherited field wins. The types said `S & S2`,
which typed that key as both brands while the schema held is the child's
alone — the lie already retired for the computed map.

Adds `MergedFields<S, S2> = Omit<S, keyof S2> & S2` and uses it at
`extend`'s return type and at its `computed` / `invariants` input
positions, so a rule's `d` reads a redeclared field honestly too. Named
and exported, as `MergedComputed` had to be: inline, the 5.9.3 emitter
copies the type parameter through unsubstituted (`TS2304`).

Measured against the deferral reason on record: the emitter writes the
alias by reference, so the billing fixture's `index.d.ts` grew 84 bytes
across two variants and all four consumer typecheck steps stay clean.

Also corrects two comments in `base.test-d.ts` — including the
pre-existing computed one this change copied — that claimed a plain
intersection would break the positive assertion. Measured: an
intersection is assignable to either constituent, so both lines compile
and the regression surfaces as the `@ts-expect-error` going unused
(`TS2578`). Documents the fields merge in `declaration.md`'s table and
its honest-surface caveat, and updates the declaration-emit name count
in `types.md` and `CLAUDE.md`.
Copilot AI lite review requested due to automatic review settings August 9, 2026 12:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns Entity.abstract(...).extend(...)’s type-level field-map merge semantics with the already child-wins runtime merge, eliminating an unsound intersection type (S & S2) that could claim a redeclared field carried both brands simultaneously. This completes the “emit-safe named alias” playbook previously applied to MergedComputed, extending it to the fields map via a new exported MergedFields helper.

Changes:

  • Introduces MergedFields<S, S2> = Omit<S, keyof S2> & S2 and switches extend’s merged-field positions from S & S2 to MergedFields<...>.
  • Exports MergedFields at the top level and as Entity.MergedFields, and wires it into the internal *Src/namespace surfaces to keep downstream declaration-emit stable.
  • Adds/updates type-level and runtime tests plus documentation to reflect “fields merge per key (child wins)” and the updated declaration-emit export list.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/entity/src/types.ts Adds MergedFields and updates AbstractEntity["extend"] generics/inputs/return type to use child-wins merged fields.
packages/entity/src/index.ts Re-exports MergedFields alongside other declaration-emit names and documents why it must remain exported.
packages/entity/src/entity.ts Threads MergedFields through the *Src aliases and Entity namespace surface.
packages/entity/src/base.test-d.ts Adds a @ts-expect-error-guarded type test proving redeclared fields are not typed as intersections on “honest” output surfaces.
packages/entity/src/base.spec.ts Adds a runtime test demonstrating redeclared field schema replacement behaves as child-wins (not parent-wins or intersect).
examples/billing-domain/src/emit-guards.ts Adds an emit guard referencing Entity.MergedFields to protect consumer declaration-emit behavior.
docs/typedoc.json Marks MergedFieldsSrc as intentionally not exported for TypeDoc purposes.
docs/reference/types.md Updates helper-types docs to include MergedFields and corrects the declaration-emit export count to nine.
docs/reference/declaration.md Updates declaration semantics table to include fields as “merged per key” and generalizes the redeclared-key caveat.
CLAUDE.md Updates repository guidance to reflect the nine declaration-emit names (including MergedComputed/MergedFields).
.changeset/merged-fields.md Adds a minor changeset describing the type-correctness fix, export rationale, and potential consumer impact.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@btravers
btravers merged commit 9018434 into main Aug 9, 2026
14 checks passed
@btravers
btravers deleted the followup/merged-fields branch August 9, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants